[release/2.1] Fix build with clang 10 #28045
Conversation
This contains a grab bag of fixes to fix the build with clang 10. - dotnet#23075 Fix missing includes in coreclr/src/debug/createdump/ - dotnet/runtime#33096 SList::Init: add missing constructor - A subset of dotnet#22129 Just the parts that introduce the THROW_DECL macro in pal.h - dotnet/runtime#32837 This fixes THROW_DECL introduce in the previous PR to work with clang, which is required in clang 10. - One new change: In a significant divergance, this commits adds more THROW_DECL macros to all the math functions to address a ton of errors pointed out when building SOS: In file included from /home/omajid/devel/dotnet/coreclr/src/ToolBox/SOS/Strike/strike.cpp:116: In file included from /home/omajid/devel/dotnet/coreclr/src/vm/hillclimbing.h:19: In file included from /home/omajid/devel/dotnet/coreclr/src/inc/complex.h:16: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/math.h:36: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/cmath:45: In file included from /usr/include/math.h:290: /usr/include/bits/mathcalls.h:53:13: error: exception specification in declaration does not match previous declaration __MATHCALL (acos,, (_Mdouble_ __x)); ^ /home/omajid/devel/dotnet/coreclr/src/pal/inc/pal.h:4395:26: note: previous declaration is here PALIMPORT double __cdecl acos(double); ^ Then, to make sure the declarations and implementations match, it adds THROW_DECL to the definitions in src/pal/src/cruntime/math.cpp Co-authored-by: Mike McLaughlin <mikem@microsoft.com> Co-authored-by: Sinan Kaya <sinan.kaya@microsoft.com> Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
|
cc @tmds |
|
I have been building coreclr using |
|
Also, CI looks busted? It checks out the source repository and immediately says testing was successful. |
|
@janvorli @tannergooding @jkotas can you review this change? |
| PALIMPORT double __cdecl sqrt(double); | ||
| PALIMPORT double __cdecl tan(double); | ||
| PALIMPORT double __cdecl tanh(double); | ||
| PALIMPORT double __cdecl acos(double) THROW_DECL; |
There was a problem hiding this comment.
Just be aware that throw() is deprecated in C++11 and is removed in C++20
This might also equally break users compiling using a different CRT implementation which is something else to keep in mind.
There was a problem hiding this comment.
Thanks for the heads up. I took this code from dotnet/runtime master:
Should we start fixing this in runtime?
|
I am sorry it took me some time to respond. I was thinking about whether instead of modifying these prototypes, we should rather ifdef out the prototypes of functions that are standard C library functions in the pal.h when PAL_STDCPP_COMPAT is defined. But I've concluded that such a thing should first be done in the runtime repo master (if it works at all). |
leecow
added
Servicing-approved
This contains a grab bag of fixes to fix the build with clang 10.
Fix missing includes in coreclr/src/debug/createdump/ #23075
Fix missing includes in coreclr/src/debug/createdump/
SList::Init: add missing constructor runtime#33096
SList::Init: add missing constructor
A subset of Coreclr gnuport #22129
Just the parts that introduce the THROW_DECL macro in pal.h
Fix re-declarations of builtin functions with clang 10 runtime#32837
This fixes THROW_DECL introduce in the previous PR to work with clang, which
is required in clang 10.
One new change:
In a significant divergance, this commits adds more THROW_DECL macros
to all the math functions to address a ton of errors pointed out when
building SOS:
Then, to make sure the declarations and implementations match, it adds
THROW_DECL to the definitions in src/pal/src/cruntime/math.cpp
Co-authored-by: Mike McLaughlin mikem@microsoft.com
Co-authored-by: Sinan Kaya sinan.kaya@microsoft.com
Co-authored-by: Tom Deseyn tom.deseyn@gmail.com